Initial support for Terrain SmoothGrid data #444

checkraisefold · 2024-08-29T03:31:15Z

DISCLAIMER: THIS IS MY FIRST ACTUAL PROJECT IN RUST. Terrible code will bleach your eyes!
Because of that, this PR also has edits by maintainers on.

This PR adds support for serialization and deserialization to rbx-types for the Terrain object SmoothGrid property data.
This fully permits the creation of external tooling for modification and generation of smooth terrain data, as Roblox Studio, along with RCCService (presumably) generate the additionally-required PhysicsGrid data at runtime/publish-time.

As a bonus, no reverse engineering of the client had to be done here; just a lot of comparing outputs with ImHex peddled by this article.
https://zeux.io/2017/03/27/voxel-terrain-storage/

Rendered spec.

Dekkonot

I'd prefer we keep the naming scheme intact and name the spec file for SmoothGrid to be smooth-grid.md.

Otherwise, I've left my comments and questions for the spec file.

docs/terrain.md

docs/smooth-grid.md

checkraisefold · 2024-09-06T13:22:02Z

further code commits are paused until structure is reviewed so I don't add the deserializer and have to immediately rewrite it

Dekkonot

Overall, I'd say that this is a good start to an encoder. The comments up above are more focused on the actual code than things like code structure.

Let's rename the terrain.rs module to smooth_grid.rs. I'd also like it if the TerrainSerializer trait could be killed off, since I don't think it adds anything. The encode and decode methods could be under the same impl blocks as the rest of the methods.

Architecturally I have some things I'd like to change but they involve files outside of this PR so I won't force them onto you.. Stuff like moving smooth_grid and material_colors into a submodule named terrain and having them share a Material enum, as an example. If you want to make those changes you're welcome to but otherwise I'll do it after this is merged.

docs/smooth-grid.md

rbx_types/src/variant.rs

rbx_xml/src/types/mod.rs

rbx_types/src/terrain.rs

checkraisefold · 2024-09-16T19:43:10Z

Architecturally I have some things I'd like to change but they involve files outside of this PR so I won't force them onto you.. Stuff like moving smooth_grid and material_colors into a submodule named terrain and having them share a Material enum, as an example. If you want to make those changes you're welcome to but otherwise I'll do it after this is merged.

i could give this a shot but I was mainly wondering how the enum should be structured with it being like this? the easiest way would be to just put it in order but you'd have to include air and some other non-colorable materials i think and then implement an error for them??

checkraisefold · 2024-09-25T00:12:57Z

i could give this a shot but I was mainly wondering how the enum should be structured with it being like this? the easiest way would be to just put it in order but you'd have to include air and some other non-colorable materials i think and then implement an error for them??

for this

Dekkonot · 2024-09-26T16:31:26Z

i could give this a shot but I was mainly wondering how the enum should be structured with it being like this? the easiest way would be to just put it in order but you'd have to include air and some other non-colorable materials i think and then implement an error for them??

There's a few ways I can imagine this working.

The first is that we just have a TerrainMaterial enum and error if you ever try to use a material that isn't allowed. This seems reasonable, but it's kind of a bummer that it would mean being unable to statically verify that a material was allowed.

Another option is to just say "to hell with it" and have multiple TerrainMaterial enums, which is how it's implemented right now. This solves the static analysis problem but it means that working with Terrain in rbx-dom requires two identically named enums, which is a terrible UX.

The final option I can see is that we have a centralized TerrainMaterial enum and then also module-specific material enums, which can be converted back and forth via from and try_from. I think I hate this idea, since it'd mean SmoothGridMaterial::try_from(TerrainMaterial::Water).unwrap() is a thing.

I'm inclined to go with the first option since it's the easiest and have the least obstructive UX possible. Moving us to a central TerrainMaterials enum is technically breaking because it involves adding variants to the one from MaterialColors, but I think I'm willing to accept it. Curious to know what @kennethloeffler thinks though.

In terms of actual implementation, I imagine that each Terrain would have its own internal variant that it used that it just tried to convert into from the central TerrainMaterials. That'd allow each module to do its own thing while still giving a consistent public interface.

Dekkonot · 2024-10-07T21:44:30Z

Can't wait for Ken for the rest of our days, so I'm gonna go ahead and say go for the plan I gave above with moving us to a central TerrainMaterials enum.

checkraisefold · 2025-01-06T00:52:19Z

No that does not compile, just pushing work so far

checkraisefold · 2025-01-06T00:55:19Z

So far my only hypothetical concern is performance - I feel like the algorithm in the encoder sucks
Decoder hopefully shouldn't be too bad in comparison at least

checkraisefold · 2025-01-07T03:08:52Z

@Dekkonot
module splitup is done, re-requesting review of the splitup before I move on to the decoder

Spec file, begin integrating it whoops fixes this ugly bit reword flat_map flat_map map make nested for loops great again The linter has spoken requested changes Spec file improvements, encoder improvements Reword Reword (2) consistency link water occupancy to shorelines chunk max const comments current changes docstring naming convention

checkraisefold · 2025-03-21T09:11:26Z

needs review - only thing missing is decoder testing

Dekkonot

This obviously still needs tests, but nothing is jumping out at me right now implementation-wise. I have some questions and suggestions but they're mostly clerical.

However, outside of the implementation there's stuff we have to do for rbx-dom to actually support reading and writing files with this type!

This needs support for reading it in rbx_xml; there's a list of conversions in rbx_xml\src\conversion.rs that you need to add an entry for SmoothGrid to.

In rbx_binary, this needs some conversions. You can take a look at my MaterialColors PR for a good idea of what that actually entails: https://github.com/rojo-rbx/rbx-dom/pull/323/files

In rbx-dom-lua, we need at least a pod in rbx_dom_lua\src\EncodedValue.lua. If we want to support live syncing it for Rojo, it also needs an entry in rbx_dom_lua\src\customProperties.lua but honestly that might be a lot of work.

Also obligatory "we need a changelog entry".

docs/smooth-grid.md

rbx_types/src/terrain/mod.rs

Dekkonot · 2025-04-16T20:28:54Z

rbx_types/src/terrain/smooth_grid.rs

+#[repr(u8)]
+#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+enum TerrainGridMaterial {


Is this repr(u8) to ensure the bit flag works or is there something else I'm missing?

Fairly sure it's to make it actually represent how it's stored in the format and also to save on memory

Dekkonot · 2025-04-16T20:33:03Z

rbx_types/src/terrain/smooth_grid.rs

+            // Shorelines uses a new water occupancy value in the voxel data. Because of this,
+            // Roblox uses a hack to avoid having to reduce their 6 bits of material ID freedom
+            // by writing voxels with a count bit set to 1 and no count. This means we have to write
+            // all voxels in the run length manually.
+            let len = to_write.len();
+            return to_write
+                .into_iter()
+                .cycle()
+                .take(len * count as usize)
+                .collect();


Would this not result in the flag being potentially written multiple times?

urmm... thats the whole point ! read the big comment

I did, thank you for the snark. Now elaborate.

because material ids are 6 bits, and the other 2 bits of the byte for the flag are taken up by write occupancy/write count toggles, they instead set the write count bit to 1 and then write a count of 0x00. then they write the water occupancy value after that. if there are multiple voxels in a row with the same water occupancy, that process is repeated for the entire amount, so the entire 3-4 byte sequence is just repeated for the entire run length. essentially run length encoding is just turned off for voxels with a nonzero water occupancy

rbx_types/src/terrain/smooth_grid.rs

checkraisefold · 2025-04-22T08:42:30Z

i have to make ChunkCoordinates/VoxelCoordinates serializable as strings that are acceptable as json keys. worst day

Dekkonot requested changes Aug 31, 2024

View reviewed changes

Dekkonot reviewed Sep 3, 2024

View reviewed changes

docs/smooth-grid.md Outdated Show resolved Hide resolved

docs/smooth-grid.md Outdated Show resolved Hide resolved

docs/smooth-grid.md Outdated Show resolved Hide resolved

checkraisefold requested a review from Dekkonot September 9, 2024 00:25

Dekkonot reviewed Sep 9, 2024

View reviewed changes

checkraisefold requested a review from Dekkonot September 25, 2024 00:12

checkraisefold force-pushed the terrain branch from c3d4683 to ca8bf6b Compare January 6, 2025 00:52

checkraisefold added 4 commits March 20, 2025 20:46

Terrain submodule

f0bdb97

further work

d8a5fe1

Vast encoder simplification, spec changes, decoder support

3b11920

checkraisefold force-pushed the terrain branch from 2189583 to 3b11920 Compare March 21, 2025 09:10

checkraisefold marked this pull request as ready for review March 21, 2025 09:10

Dekkonot reviewed Apr 16, 2025

View reviewed changes

checkraisefold added 4 commits April 22, 2025 00:01

partial review

1292c30

partial review part 2 electric boogaloo

a7c0f4e

Merge branch 'rojo-rbx:master' into terrain

6b37f3c

changelogs

e103ec4

Initial support for Terrain SmoothGrid data #444

Are you sure you want to change the base?

Initial support for Terrain SmoothGrid data #444

Uh oh!

Conversation

checkraisefold commented Aug 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dekkonot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

checkraisefold commented Sep 6, 2024

Uh oh!

Dekkonot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

checkraisefold commented Sep 16, 2024

Uh oh!

checkraisefold commented Sep 25, 2024

Uh oh!

Dekkonot commented Sep 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dekkonot commented Oct 7, 2024

Uh oh!

checkraisefold commented Jan 6, 2025

Uh oh!

checkraisefold commented Jan 6, 2025

Uh oh!

checkraisefold commented Jan 7, 2025

Uh oh!

checkraisefold commented Mar 21, 2025

Uh oh!

Dekkonot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Dekkonot Apr 16, 2025

Choose a reason for hiding this comment

Uh oh!

checkraisefold Apr 21, 2025

Choose a reason for hiding this comment

Uh oh!

Dekkonot Apr 16, 2025

Choose a reason for hiding this comment

Uh oh!

checkraisefold Apr 21, 2025

Choose a reason for hiding this comment

Uh oh!

Dekkonot Apr 21, 2025

Choose a reason for hiding this comment

Uh oh!

checkraisefold Apr 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

checkraisefold commented Aug 29, 2024 •

edited

Loading

Dekkonot commented Sep 26, 2024 •

edited

Loading